chore: chart polish, E2E speedup, and implementation guide - #599

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/e2e/global-setup.ts`:
- Around line 267-279: The comment and behavior mismatch: current code only
checks E2E_SKIP_BUILD and will skip building even when .next/BUILD_ID is
missing, causing next start to fail; update the build block in global-setup.ts
to check both that process.env.E2E_SKIP_BUILD is set AND that the BUILD_ID file
exists before skipping (i.e., only skip when BUILD_ID is present), and if
E2E_SKIP_BUILD is set but BUILD_ID is missing, either run the build or throw a
clear error; also remove the unnecessary dynamic await
import("node:child_process") and use the existing static import (replace
execSync import with the already-imported execSync used alongside spawn) so
execSync is available without dynamic import.

In `@app/src/plugins/sunburst/settings.ts`:
- Line 9: The maxLabelDepth schema uses z.coerce.number() which silently turns
blank inputs into 0 and bypasses the default; update the maxLabelDepth schema to
preprocess blank/null/empty-string into undefined (using z.preprocess) before
coercion, then validate with z.number().int().min(1).max(10).default(2) (or
another appropriate max) so empty inputs fall back to the default and values are
constrained; modify the maxLabelDepth line to use
z.preprocess(...)->z.number().int().min(1).max(10).default(2).

In `@component/src/charts/gauge-chart.tsx`:
- Around line 90-102: The progressColor currently ignores thresholdZones; change
progressColor computation so it first uses resolvedColor (from
resolveItemColor(point.value, stylingRules, paramValues)), and if that is
null/undefined then if hasCustomZones is true derive the active progress color
from thresholdZones using the current value (point.value) or the same selection
logic used for trackColor's zones, otherwise fall back to the default "#5470c6";
update references to progressColor, hasCustomZones, thresholdZones and
resolvedColor to implement this selection.

In `@component/src/charts/sunburst-chart.tsx`:
- Around line 26-27: The prop maxLabelDepth currently treats 0 as a real value
due to using nullish coalescing (maxLabelDepth ?? 2), which makes 0 hide labels;
change the normalization to treat 0 as the "auto" sentinel so the component
falls back to the default (e.g., 2) when maxLabelDepth is 0 or undefined. Locate
where maxLabelDepth is normalized (references: the prop name maxLabelDepth and
any internal variable like maxLabelDepthUsed / normalizedMaxLabelDepth or the
logic around lines 26 and 61-65) and update it to: if maxLabelDepth === 0 then
use the auto/default value, otherwise use the provided positive number (or the
existing ?? default). Ensure the comparison logic that decides whether to render
a label uses the normalized value so depth <= normalized behaves correctly.
- Around line 143-157: The series-level label configuration in the Sunburst
chart is ignoring the showLabels prop: update the top-level series label.show
(currently using only !compact) to respect showLabels (i.e., label.show =
showLabels && !compact) and also change the emphasis.block's label.show
(currently hardcoded true) to use showLabels (e.g., emphasis.label.show =
showLabels) so hover/emphasis respects the prop; adjust in the series object
where label, minAngle, and emphasis are defined to be consistent with the
existing levels check that uses showLabels && !compact.

---

Outside diff comments:
In `@component/src/charts/gauge-chart.tsx`:
- Around line 1-17: The GaugeChart module initializes echarts at module scope
which runs during SSR; wrap the exported GaugeChart component with
next/dynamic(..., { ssr: false }) in this library so the echarts.use(...) and
GaugeChart rendering only run on the client. Replace/export the GaugeChart (the
component defined in gauge-chart.tsx and re-exported from charts/index.ts) as a
dynamically imported component via next/dynamic with ssr: false, or move the
echarts.use(...) calls into the client-only wrapper so that BaseChart/GaugeChart
are only imported/executed on the client.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/src/plugins/gantt/settings.ts`:
- Line 11: The barBorderRadius schema currently uses
z.coerce.number().default(2) which allows negative numbers; update the
validation for the barBorderRadius symbol to enforce non-negative values by
adding .min(0) (e.g. z.coerce.number().min(0).default(2)); optionally consider
adding an upper bound with .max(...) if you want to cap extreme values to avoid
ECharts rendering issues.

In `@app/src/plugins/gantt/transform.ts`:
- Around line 89-104: parseTime currently uses a fixed 1e12 threshold to decide
seconds vs milliseconds which misclassifies pre-2001 millisecond timestamps;
update parseTime to accept an optional unit hint (e.g., unitHint?: "seconds" |
"milliseconds") and use that when provided, otherwise keep the heuristic but add
a clear comment about its limitations; update the function signature
parseTime(value: unknown, unitHint?: "seconds" | "milliseconds") and apply
unitHint to choose whether to multiply by 1000 (if "seconds") or treat as ms (if
"milliseconds"), leaving the existing fallback heuristic only when unitHint is
absent.
- Around line 76-82: Normalize the progress value as you already do (using
progressKey and local p from row[progressKey]) but then clamp it into the 0..1
range before assigning to item.progress; i.e., after converting percentages (p >
1 ? p/100 : p) apply Math.max(0, Math.min(1, normalized)) so negatives become 0
and overly large percentages (e.g., 150) are capped at 1.

In `@component/src/charts/gantt-chart.tsx`:
- Around line 153-169: The progress overlay uses the same corner radius for all
corners (shape.r = barBorderRadius) which causes the overlay's trailing edge to
appear rounded mid-bar; update the overlay rectangle creation in the render code
that builds the overlay (the block that checks showProgress && progress > 0) to
use a corner-radius array that only rounds the leading/left corners when
progress < 1 (e.g., left-top and left-bottom = barBorderRadius, right corners =
0), and keep full rounding when progress >= 1; additionally clamp progress to
the [0,1] range once when constructing seriesData (the place that currently
feeds Math.round(Number(v[5]) * 100) in the tooltip) so both rendering and
tooltip percent calculations agree.

---

Nitpick comments:
In `@app/src/components/widget-editor/__tests__/transform-editor.test.tsx`:
- Around line 59-70: Tests currently mock `@radix-ui/react-select` primitives
(Item, ItemText, ItemIndicator) and the suite misses an assertion ensuring the
collapsed Select trigger only shows the label (not label + description); add a
focused test in transform-editor.test.tsx that renders the transform-editor,
selects the relevant item via the mocked SelectPrimitive.Item, and asserts the
selected trigger's accessible text equals "Filter" (and does not contain the
description like "Remove rows matching a condition") so regressions reverting to
wrapped SelectItem (label+description) are caught.

In `@app/src/components/widget-editor/transform-editor.tsx`:
- Line 13: The trigger was showing both label and description because
`@neoboard/components`' SelectItem wraps all children in ItemText; switch those
option usages to use SelectPrimitive.Item so only t.label is wrapped in ItemText
for the trigger. Update occurrences in transform-editor.tsx (the SelectItem
imports/usages) to render options with SelectPrimitive.Item (and keep the
description as a sibling span inside the Item) so the collapsed trigger shows
only t.label; consider mirroring this change for the same pattern at the other
occurrences mentioned (lines ~526-540) to keep behavior consistent.

In `@app/src/plugins/__tests__/plugin-options.test.ts`:
- Around line 6-24: The mock OPTION_COUNTS object used by fakeGetChartOptions is
missing a "gantt" entry and the `@neoboard/components` test mock doesn't export a
GanttChart stub; update OPTION_COUNTS (refer to the constant named
OPTION_COUNTS) to include a "gantt": <number> entry (choose a realistic count
like 4–8) so fakeGetChartOptions("gantt") returns the expected options array,
and add a GanttChart export to the `@neoboard/components` mock (the same module
that exports BarChart/LineChart/etc.) so the gantt plugin’s dynamic import has a
matching stub (keep the stub shape consistent with other chart stubs).

In `@app/src/plugins/gantt/transform.ts`:
- Around line 13-18: Change the transformToGanttData signature and its returns
to a concrete array type instead of unknown: replace the return type unknown
with GanttDataItem[] (or Array<Record<string, unknown>> if GanttDataItem isn't
available), import or define the GanttDataItem type, and ensure all early
returns (e.g., when records are empty or keys.length < 3) return an empty array
of that type; update callers (such as component.tsx) to remove unnecessary casts
now that transformToGanttData produces the proper typed array.

In `@component/src/charts/__tests__/gantt-chart.test.tsx`:
- Around line 119-134: The test currently only checks seriesData length and
should verify that stylingRules are applied to matching tasks; update the test
in gantt-chart.test.tsx to locate the data entries in optionsCall.series[0].data
that correspond to tasks with column value "Phase 2" (use the same sampleData
field name used by GanttChart) and assert those entries carry the expected color
metadata (e.g., an itemStyle.color or color property equal to "#ff0000");
reference GanttChart, stylingRules, mockSetOption, seriesData and renderItem
when locating the code paths to validate the per-item color metadata rather than
only length.

In `@component/src/charts/gantt-chart.tsx`:
- Around line 253-262: The current series config sidesteps type-checking by
using "renderItem: renderItem as never"; instead import and use ECharts'
CustomSeriesRenderItem type and type the renderItem function accordingly (the
function currently defined around lines 104–115), then remove the "as never"
cast in the series object (where renderItem is assigned) so the series entry
uses the properly typed renderItem; ensure you add "import type {
CustomSeriesRenderItem } from 'echarts'" and annotate the renderItem signature
with that type to catch API drift.

In `@component/stories/charts/gantt-chart.stories.tsx`:
- Around line 167-172: The story's data array uses Math.random() to compute task
end dates which makes the LargeDataset non-deterministic; replace the
Math.random() call used in the end calculation (the Array.from(... => ({ task:
`Task ${i + 1}`, start: day(...), end: day(...), category: ... }))) with a
deterministic variation—either implement a small seeded PRNG and call it for
each index or use a deterministic formula like (i * prime) % N to produce the
variable duration—so the end value uses day(i * 2 + deterministicOffset + 3)
instead of Math.random() to ensure stable Storybook/snapshot outputs while
keeping varied durations.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/e2e/global-setup.ts`:
- Around line 272-297: The code currently treats process.env.E2E_SKIP_BUILD as a
boolean which makes values like "0" or "false" still count as truthy and skip
the build; update the checks around process.env.E2E_SKIP_BUILD (the block that
decides whether to skip or run execSync("npx next build", { cwd: appDir, stdio:
"inherit", env: serverEnv })) to explicitly parse/normalize the env var (e.g.
treat "0", "false", "no" as falsy and "1", "true", "yes" as truthy) before
branching; use the normalized boolean in both the initial hasCachedBuild guard
and the later if/else that logs skipping vs building so a user setting
E2E_SKIP_BUILD=0 will not skip the rebuild.

In `@component/src/charts/gantt-chart.tsx`:
- Around line 224-237: The tooltip formatter can call echarts.format.encodeHTML
with an undefined name when v[0] is out of range; in the formatter function
(formatter: (params: unknown) => { ... }) validate that params.value is an array
and that taskIndex = Number(v[0]) is an integer inside the bounds of taskNames,
then derive name = taskNames[taskIndex] ?? 'Unknown task' (or empty string)
before calling echarts.format.encodeHTML; also guard accesses to v[1..5] (coerce
to numbers or provide safe defaults) so formatDuration(v[3]) and progress
calculation never receive undefined.

In `@component/src/charts/gauge-chart.tsx`:
- Around line 102-105: The lookup for thresholdColor assumes thresholdZones are
ordered but parseGaugeThresholdZones preserves user order; to fix, ensure the
zones used for color selection are sorted ascending by their stop value before
performing the find — e.g., sort the thresholdZones array (the one used in
gauge-chart.tsx where thresholdColor is computed) by the numeric stop ([stop])
so thresholdZones.find(([stop]) => normalizedValue <= stop) yields the correct
zone; alternatively, enforce sorting inside parseGaugeThresholdZones so any
consumer (including thresholdColor computation) receives zones in ascending
order.

---

Duplicate comments:
In `@component/src/charts/gantt-chart.tsx`:
- Around line 113-123: When building seriesData in the data.map callback, clamp
the progress value before storing it (replace item.progress ?? 0 with a clamped
value like Math.max(0, Math.min(item.progress ?? 0, 1))) so the stored v[5] used
by both the rendered overlay and the tooltip stays consistent; update the
seriesData construction (the map creating value: [..., item.progress ?? 0, ...])
to use the clamped progress and leave resolvedColors/itemStyle unchanged.

---

Nitpick comments:
In `@app/src/plugins/gantt/transform.ts`:
- Line 13: The function transformToGanttData currently returns unknown and
should instead return a well-typed array; define a GanttDatum type/interface
with required fields { task: string; start: Date | string; end: Date | string;
category?: string; progress?: number } and change transformToGanttData signature
to return GanttDatum[]; update any callers (component.tsx and the transforms
barrel) to use GanttDatum instead of casting, and ensure any internal
parsing/conversion within transformToGanttData produces values matching those
types (e.g., convert start/end to Date or a consistent string format).

In `@component/src/charts/gauge-chart.tsx`:
- Around line 109-111: The code uses "as never" to silence the type system for
trackColor; instead declare/use a proper ECharts color type (e.g., type
EChartsColor = string | [number, string][]) and apply it to trackColor and/or
thresholdZones so the union expression matches ECharts' lineStyle.color; update
the expression in gauge-chart.tsx that sets trackColor (referencing trackColor,
hasCustomZones, thresholdZones) to use this EChartsColor type or cast to
EChartsColor rather than using "as never", or annotate the original
thresholdZones declaration with EChartsColor so no unsafe cast is needed.